app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt (1)

183-185: Re-throw CancellationException to preserve structured concurrency.

catch (e: Exception) will intercept kotlinx.coroutines.CancellationException (which extends IllegalStateException → RuntimeException → Exception). If the lifecycleScope is cancelled while store.grantPermission(...) is executing inside withContext(Dispatchers.IO), the cancellation signal is swallowed, the error path logs it as a routine failure, and execution falls through to finish() — breaking Kotlin's structured-concurrency contract.

The original catch (t: Throwable) had the same flaw; this change is the right moment to fix it.

♻️ Proposed fix

-            } catch (e: Exception) {
-                if (BuildConfig.DEBUG) Log.e(TAG, "Failed to persist permission: ${e::class.simpleName}")
+            } catch (e: Exception) {
+                if (e is kotlinx.coroutines.CancellationException) throw e
+                if (BuildConfig.DEBUG) Log.e(TAG, "Failed to persist permission: ${e::class.simpleName}")
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip46/Nip46ApprovalActivity.kt` around
lines 183 - 185, The catch block in Nip46ApprovalActivity (around the
store.grantPermission call executed from
lifecycleScope/withContext(Dispatchers.IO)) is catching Exception and thus
swallowing kotlinx.coroutines.CancellationException; adjust the handler so
CancellationException is re-thrown (e.g., detect
kotlin.coroutines.cancellation.CancellationException or Throwable of that type
and throw it) before logging/handling other exceptions so structured concurrency
is preserved and cancellations propagate correctly.

app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt (1)

262-297: Consider disabling the "Add" button or showing a loading indicator during async validation.

When the user taps "Add", the isInternalHost check (single relay) or addBunkerRelays (bunker URL) runs asynchronously. During this time the button remains tappable with no visual feedback. A brief loading state would improve UX and prevent duplicate submissions, though the outcome is idempotent so this isn't a correctness issue.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip46/BunkerScreen.kt` around lines 262 -
297, Add a transient loading state to prevent duplicate taps and show feedback:
introduce a mutable Boolean (e.g. isAdding/isLoading) that is checked at the
start of the TextButton onClick and used to disable the Add button and show a
small loading indicator in the button. Set isLoading = true immediately before
launching async work (both the scope.launch branch that calls addBunkerRelays
and the withContext(Dispatchers.IO) BunkerConfigStore.isInternalHost branch),
and ensure you reset isLoading = false in all completion paths
(onSuccess/onFailure, internal host branch, and after
onRelaysUpdated/dismissDialog) so the button re-enables and the indicator hides;
keep existing variables like newRelayUrl, addBunkerRelays, validateRelayUrl,
onRelaysUpdated and dismissDialog intact.

app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt (1)

193-208: AppPermissionsListContent has a large parameter surface (14 params) with embedded business logic.

This composable receives stores and coroutineScope directly, coupling UI to the data layer. Consider extracting the business operations into callbacks passed from the parent in a future refactor, similar to how ImportShareInputFields only receives display state and event callbacks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip55/AppPermissionsScreen.kt` around
lines 193 - 208, AppPermissionsListContent currently takes many data-layer types
(SignPolicyStore, PermissionStore, CoroutineScope) and implements business
logic; refactor by removing stores and coroutineScope from its signature and
replace them with explicit UI event callbacks and minimal display state (e.g.,
onLoadPermissions(), onRevokeAllRequested(), onUpdateExpiry(expiry: Long),
onSaveSettings(settings: Nip55AppSettings?), onChangeAppState(state: AppState),
plus any simple data lists or flags needed for rendering), then move the actual
business operations (calls into SignPolicyStore/PermissionStore and
coroutineScope launches) up to the parent caller that owns those stores; update
callers to pass the new callbacks and keep AppPermissionsListContent as a pure
UI/composable that only receives display state and event handlers.

app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt (2)

65-65: Evicting rate-limit history should also clean per‑client state

When the cap is hit, only clientRequestHistory is evicted. clientBackoffUntil and clientConsecutiveRequests can still grow without bound and retain stale backoff for evicted clients. Consider clearing related per‑client state when evicting.

♻️ Suggested tweak to evict related state

 if (clientRequestHistory.size >= MAX_TRACKED_CLIENTS && !clientRequestHistory.containsKey(clientPubkey)) {
-    clientRequestHistory.keys.firstOrNull()?.let { clientRequestHistory.remove(it) }
+    clientRequestHistory.keys.firstOrNull()?.let { evicted ->
+        clientRequestHistory.remove(evicted)
+        clientBackoffUntil.remove(evicted)
+        clientConsecutiveRequests.remove(evicted)
+    }
 }

Also applies to: 171-173

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt` at line 65, The
eviction currently only removes entries from clientRequestHistory
(MAX_TRACKED_CLIENTS) but leaves related per-client state in clientBackoffUntil
and clientConsecutiveRequests, letting those maps grow and preserving stale
backoff; update the eviction logic (the block handling MAX_TRACKED_CLIENTS) to
also delete the same client keys from clientBackoffUntil and
clientConsecutiveRequests whenever you evict from clientRequestHistory (ensure
you use the same clientId/key used by clientRequestHistory removal), and apply
the same cleanup in the other eviction site that mirrors this logic so all three
maps stay in sync.

---

41-41: Consider surfacing timeout vs. "no decision"

withTimeoutOrNull returns null on timeout, which is indistinguishable from "no stored decision." If you want visibility into timeouts, consider withTimeout + TimeoutCancellationException logging.

🔍 Optional timeout visibility

 return runCatching {
     runBlocking(Dispatchers.IO) {
-        withTimeoutOrNull(5_000L) {
-            store.getPermissionDecision(callerPackage, requestType, eventKind)
-        }
+        try {
+            withTimeout(5_000L) {
+                store.getPermissionDecision(callerPackage, requestType, eventKind)
+            }
+        } catch (e: TimeoutCancellationException) {
+            if (BuildConfig.DEBUG) Log.w(TAG, "Permission lookup timed out")
+            null
+        }
     }
 }.getOrNull()

Also applies to: Nip55ContentProvider.kt

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip46/BunkerService.kt` at line 41, The
current use of withTimeoutOrNull in BunkerService.kt (and similarly in
Nip55ContentProvider.kt) hides whether a null result came from a timeout or from
"no stored decision"; replace with withTimeout around the same call and add a
catch for kotlinx.coroutines.TimeoutCancellationException in the surrounding
coroutine scope to explicitly log/handle timeouts (include function names where
used, e.g., the method that calls withTimeoutOrNull in BunkerService and the
corresponding provider method in Nip55ContentProvider), so the catch can emit a
clear processLogger/error or metrics entry indicating a timeout vs a genuine
null decision; keep the original null-handling logic separate from the timeout
handling.

app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt (1)

225-266: Optional: memoize grouped permissions to reduce recomposition work.

groupBy runs on every recomposition. If the list is large, consider memoizing by permissions to avoid repeated grouping.

♻️ Suggested tweak

-    val groupedPermissions = permissions.groupBy { it.callerPackage }
+    val groupedPermissions = remember(permissions) {
+        permissions.groupBy { it.callerPackage }
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/src/main/kotlin/io/privkey/keep/nip55/PermissionsManagementScreen.kt`
around lines 225 - 266, The grouping of permissions in PermissionsGroupedList is
recomputed on every recomposition; replace the direct call to
permissions.groupBy { it.callerPackage } with a memoized value (e.g., use
remember or derivedStateOf keyed on permissions) so groupedPermissions is only
recalculated when the permissions list changes, leaving the rest of the function
(LazyColumn, AppPermissionHeader, PermissionCard) unchanged.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro